From dff6ba1a0dd1440851e1d708b33a8f666ea283b9 Mon Sep 17 00:00:00 2001 From: "sos22@labyrinth.cl.cam.ac.uk" Date: Tue, 8 Jul 2003 10:38:14 +0000 Subject: [PATCH] bitkeeper revision 1.294 (3f0a9f16gdwvRs89xXMLxr3DvV1dgg) Switch from using magic messages to map domain memory to using a magic ioctl. There's something very weird in this code: if userspace forgets to unmap the memory, the next time it's called, it crashes. It looks like the old code had the same bug, so I'm going to pretend it isn't there for a little while. --- tools/internal/xi_build.c | 64 +++++++++++-------- xen/include/hypervisor-ifs/dom0_ops.h | 25 ++++++-- .../arch/xeno/drivers/dom0/dom0_core.c | 58 +++++++++++++++-- 3 files changed, 110 insertions(+), 37 deletions(-) diff --git a/tools/internal/xi_build.c b/tools/internal/xi_build.c index a9dec00fcb..4f9ef136ba 100644 --- a/tools/internal/xi_build.c +++ b/tools/internal/xi_build.c @@ -12,6 +12,7 @@ #include #include #include +#include #include "hypervisor_defs.h" #include "dom0_ops.h" @@ -47,23 +48,23 @@ static void dbstatus(char * msg) /* clean up domain's memory allocations */ static void dom_mem_cleanup(dom_mem_t * dom_mem) { - char mem_path[MAX_PATH]; - int mem_fd; - - /* open the domain's /proc mem interface */ - sprintf(mem_path, "%s%s%s%s%d%s%s", "/proc/", PROC_XENO_ROOT, "/", - PROC_DOM_PREFIX, dom_mem->domain, "/", PROC_DOM_MEM); + int fd; + struct dom0_unmapdommem_args argbuf; - mem_fd = open(mem_path, O_WRONLY); - if(mem_fd < 0){ + fd = open("/proc/xeno/dom0_cmd", O_WRONLY); + if(fd < 0){ perror(PERR_STRING); } - if(write(mem_fd, (dom_mem_t *)dom_mem, sizeof(dom_mem_t)) < 0){ - dbstatus("Error unmapping domain's memory.\n"); + argbuf.vaddr = dom_mem->vaddr; + argbuf.start_pfn = dom_mem->start_pfn; + argbuf.tot_pages = dom_mem->tot_pages; + + if (ioctl(fd, IOCTL_DOM0_UNMAPDOMMEM, &argbuf) < 0) { + dbstatus("Error unmapping domain's memory.\n"); } - close(mem_fd); + close(fd); } /* ask dom0 to export domains memory through /proc */ @@ -121,21 +122,30 @@ static unsigned long get_vaddr(unsigned int dom) static int map_dom_mem(unsigned long pfn, int pages, int dom, dom_mem_t * dom_mem) { - - if(setup_dom_memmap(pfn, pages, dom)){ - perror(PERR_STRING); - return -1; - } - - dom_mem->domain = dom; - dom_mem->start_pfn = pfn; - dom_mem->tot_pages = pages; - if((dom_mem->vaddr = get_vaddr(dom)) == 0){ - dberr("Error mapping dom memory."); - return -1; - } - - return 0; + struct dom0_mapdommem_args argbuf; + int fd; + + argbuf.domain = dom; + argbuf.start_pfn = pfn; + argbuf.tot_pages = pages; + + fd = open("/proc/xeno/dom0_cmd", O_RDWR); + if (fd < 0) { + perror("openning /proc/xeno/dom0_cmd"); + return -1; + } + + dom_mem->domain = dom; + dom_mem->start_pfn = pfn; + dom_mem->tot_pages = pages; + dom_mem->vaddr = ioctl(fd, IOCTL_DOM0_MAPDOMMEM, &argbuf); + + if (dom_mem->vaddr == -1) { + perror("mapping domain memory"); + return -1; + } + + return 0; } /* open kernel image and do some sanity checks */ @@ -220,7 +230,7 @@ static dom_meminfo_t *setup_guestos(int dom, int kernel_fd, int initrd_fd, pgt_updates = (page_update_request_t *)dom_mem->vaddr; alloc_index = dom_mem->tot_pages - 1; - memset(meminfo, 0, sizeof(meminfo)); + memset(meminfo, 0, sizeof(*meminfo)); memcpy(page_array, (void *)dom_mem->vaddr, dom_mem->tot_pages * 4); diff --git a/xen/include/hypervisor-ifs/dom0_ops.h b/xen/include/hypervisor-ifs/dom0_ops.h index 67c69008d6..6286f11b98 100644 --- a/xen/include/hypervisor-ifs/dom0_ops.h +++ b/xen/include/hypervisor-ifs/dom0_ops.h @@ -24,8 +24,6 @@ #define MAX_CMD_LEN 256 #define MAX_DOMAIN_NAME 16 -#define IOCTL_DOM0_CREATEDOMAIN _IOC(_IOC_NONE, 'x', 0, 0) - typedef struct dom0_newdomain_st { unsigned int domain; // return parameter @@ -114,12 +112,31 @@ typedef struct dom0_op_st } dom0_op_t; #endif -/* Arguments to the CREATEDOMAIN ioctl on /proc/xeno/dom0_cmd. - Probably belongs in a Linux include file somewhere... */ +/* These really belong in a Linux header file somewhere. + XXX. +*/ +#define IOCTL_DOM0_CREATEDOMAIN _IOC(_IOC_READ, 'x', 0, sizeof(struct dom0_createdomain_args)) +#define IOCTL_DOM0_MAPDOMMEM _IOC(_IOC_READ, 'x', 1, sizeof(struct dom0_mapdommem_args)) +#define IOCTL_DOM0_UNMAPDOMMEM _IOC(_IOC_READ, 'x', 2, sizeof(struct dom0_unmapdommem_args)) + struct dom0_createdomain_args { unsigned int kb_mem; const char *name; }; +struct dom0_mapdommem_args +{ + unsigned int domain; + unsigned start_pfn; + unsigned tot_pages; +}; + +struct dom0_unmapdommem_args +{ + unsigned long vaddr; + unsigned long start_pfn; + unsigned long tot_pages; +}; + #endif diff --git a/xenolinux-2.4.21-sparse/arch/xeno/drivers/dom0/dom0_core.c b/xenolinux-2.4.21-sparse/arch/xeno/drivers/dom0/dom0_core.c index 60ea20a649..0e1a981605 100644 --- a/xenolinux-2.4.21-sparse/arch/xeno/drivers/dom0/dom0_core.c +++ b/xenolinux-2.4.21-sparse/arch/xeno/drivers/dom0/dom0_core.c @@ -157,6 +157,8 @@ static ssize_t dom_mem_write(struct file * file, const char * buff, { dom_mem_t mem_data; + printk("dom_mem_write called: Shouldn't happen.\n"); + copy_from_user(&mem_data, (dom_mem_t *)buff, sizeof(dom_mem_t)); if(direct_disc_unmap(mem_data.vaddr, mem_data.start_pfn, @@ -178,6 +180,9 @@ static ssize_t dom_mem_read(struct file * file, char * buff, size_t size, loff_t /* remap the range using xen specific routines */ + printk("Calling direct_mmap with pfn %x, tot pages %x.\n", + mem_data->pfn, mem_data->tot_pages); + addr = direct_mmap(mem_data->pfn << PAGE_SHIFT, mem_data->tot_pages << PAGE_SHIFT, prot, MAP_DISCONT, mem_data->tot_pages); copy_to_user((unsigned long *)buff, &addr, sizeof(addr)); @@ -204,6 +209,7 @@ static int dom_map_mem(unsigned int dom, unsigned long pfn, int tot_pages) /* check if there is already an entry for mem and if so * remove it. */ + /* XXX does this not leak the memdata? */ remove_proc_entry("mem", pd); /* create new entry with parameters describing what to do @@ -265,6 +271,10 @@ static int dom0_cmd_write(struct file *file, const char *buffer, size_t size, { ret = dom_map_mem(op.u.dommem.domain, op.u.dommem.start_pfn, op.u.dommem.tot_pages); + /* This is now an ioctl, and shouldn't be being written to + the command file. */ + // printk("map_dom_mem dom0_cmd used!\n"); + // ret = -EOPNOTSUPP; } else if ( op.cmd == DO_PGUPDATES ) { @@ -387,20 +397,16 @@ static int handle_dom0_cmd_createdomain(unsigned long data) dom0_op_t op; int ret; - if (copy_from_user(&argbuf, (void *)data, sizeof(argbuf))) { - printk("fault getting argbuf.\n"); + if (copy_from_user(&argbuf, (void *)data, sizeof(argbuf))) return -EFAULT; - } op.cmd = DOM0_CREATEDOMAIN; op.u.newdomain.domain = -666; op.u.newdomain.memory_kb = argbuf.kb_mem; op.u.newdomain.num_vifs = 0; /* Not used anymore, I hope... */ namelen = strnlen_user(argbuf.name, MAX_DOMAIN_NAME); - if (copy_from_user(op.u.newdomain.name, argbuf.name, namelen + 1)) { - printk("Fault getting domain name\n"); + if (copy_from_user(op.u.newdomain.name, argbuf.name, namelen + 1)) return -EFAULT; - } /* Error checking? The old code deosn't appear to do any, and I can't see where the return values are documented... */ @@ -423,6 +429,42 @@ static int handle_dom0_cmd_createdomain(unsigned long data) return ret; } +static unsigned long handle_dom0_cmd_mapdommem(unsigned long data) +{ + struct dom0_mapdommem_args argbuf; + unsigned long addr; + + if (copy_from_user(&argbuf, (void *)data, sizeof(argbuf))) + return -EFAULT; + /* This seems to be assuming that the root of the page table is in + the first frame of the new domain's physical memory? */ + /* XXX do I really mean this? */ + /* XXX what happens if userspace forgets to do the unmap? */ + printk("direct_maping w/ start pfn %x, tot_pages %x.\n", + argbuf.start_pfn, argbuf.tot_pages); + + addr = direct_mmap(argbuf.start_pfn << PAGE_SHIFT, + argbuf.tot_pages << PAGE_SHIFT, + PAGE_SHARED, + MAP_DISCONT, + argbuf.tot_pages); + + printk("Picked vaddr %x.\n", addr); + + return addr; +} + +static int handle_dom0_cmd_unmapdommem(unsigned long data) +{ + struct dom0_unmapdommem_args argbuf; + + if (copy_from_user(&argbuf, (void *)data, sizeof(argbuf))) + return -EFAULT; + + return direct_disc_unmap(argbuf.vaddr, argbuf.start_pfn, + argbuf.tot_pages); +} + static int dom0_cmd_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long data) { @@ -430,6 +472,10 @@ static int dom0_cmd_ioctl(struct inode *inode, struct file *file, switch (cmd) { case IOCTL_DOM0_CREATEDOMAIN: return handle_dom0_cmd_createdomain(data); + case IOCTL_DOM0_MAPDOMMEM: + return handle_dom0_cmd_mapdommem(data); + case IOCTL_DOM0_UNMAPDOMMEM: + return handle_dom0_cmd_unmapdommem(data); default: printk("Unknown dom0_cmd ioctl!\n"); return -EINVAL; -- 2.30.2